[GeoMechanicsApplication] Move all non-template code to .cpp files and make sure any .h file does not contain any implementations (processes) #14075

markelov208 · 2025-12-18T12:44:37Z

📝 Description
A brief description of the PR.

split hpp files into header and cpp files
renamed some headers into hpp if they include template functions

…ocess

…pressure_process

…s hpp

…-cpp-processes

rfaasse

Thanks for the efforts of splitting classes into header + source, improving the 'include health' in a lot of these classes and also making the changes in a way that makes diffing as easy as possible (i.e. you kept the same order in the functions, that's much appreciated 😄 ).

I didn't do an in-depth review of all code, but mainly checked it stayed the same and all implementations were moved out of the headers. I only have a few minor comments about forward declares, includes and some left-over implementations in .h files.

...oMechanicsApplication/custom_processes/apply_boundary_phreatic_line_pressure_table_process.h

rfaasse · 2025-12-24T07:14:56Z

applications/GeoMechanicsApplication/custom_processes/apply_c_phi_reduction_process.h

 {

 class Model;
+class Parameters;


Here the forward declare does work, because the Parameters are passed as reference 👍

applications/GeoMechanicsApplication/custom_processes/apply_component_table_process.h

rfaasse · 2025-12-24T07:26:21Z

.../GeoMechanicsApplication/custom_processes/apply_constant_interpolate_line_pressure_process.h

+/// output stream function
+inline std::ostream& operator<<(std::ostream& rOStream, const ApplyConstantInterpolateLinePressureProcess& rThis)
+{
+    rThis.PrintInfo(rOStream);
+    rOStream << std::endl;
+    rThis.PrintData(rOStream);
+
+    return rOStream;
+}


Could this also be moved to the cpp?

I think it is possible but then it could not be inline, right? As far as I remember a compiler makes a decision and it can ignore inline statements.

removed these operators. Once Anne asked to remove such lines.

That's also a solution, fine with me!

...echanicsApplication/custom_processes/apply_constant_phreatic_multi_line_pressure_process.cpp

applications/GeoMechanicsApplication/custom_processes/apply_normal_load_table_process.h

applications/GeoMechanicsApplication/custom_processes/apply_write_result_scalar_process.h

.../GeoMechanicsApplication/custom_processes/apply_phreatic_multi_line_pressure_table_process.h

...eoMechanicsApplication/custom_processes/deactivate_conditions_on_inactive_elements_process.h

…-cpp-processes

…-cpp-processes # Conflicts: # applications/GeoMechanicsApplication/custom_workflows/dgeoflow.h # applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp

rfaasse

Thank you for processing the review comments. I only have a single suggestion, that you should ignore, unless there are some other changes you'd also like to do. Ready to go from my side!

rfaasse · 2026-01-16T08:00:48Z

applications/GeoMechanicsApplication/custom_processes/apply_write_result_scalar_process.h

    /// Execute method is used to execute the ApplyWriteScalarProcess algorithms.
-    void Execute() override;
+    void Execute() override {
+        // to avoid using the base class function
+    };


The Execute method of the base is also just an empty function, so I don't think we need to override it, we could just remove the function. It's not enough to block the PR for though, so please only do it if there's something else you'd like to change as well.

rfaasse · 2026-01-16T08:12:53Z

...eoMechanicsApplication/custom_processes/apply_constant_interpolate_line_pressure_process.cpp

-        for (unsigned int j = 0; j < BoundaryNodes.size(); ++j) {
-            if (Id == BoundaryNodes[j]) {
-                mBoundaryNodes[iPosition++] = &rNode;
+        const auto Id = static_cast<int>(rNode.Id());


For now, I think the static casts are fine. However, I think in the longer term, it would be a good idea to just make sure everything in this class is in terms of std::size_t, since the Id() functions return this. But that's out of scope of this PR and probably also needs to be discussed with others, so let's leave it for now (same for the other casts in this file)

rfaasse · 2026-01-16T08:18:44Z

...eoMechanicsApplication/custom_processes/apply_constant_interpolate_line_pressure_process.cpp

-                ElementIDs[iPoint][i] = iElementID;
-            }
-        }
+    constexpr int ID_UNDEFINED = -1;


Out of scope for this PR (and of course it was already there), but working like this with 'undefined ids' should really be avoided. We can refactor this later (first adding unit tests, then making changes)

rfaasse · 2026-01-16T08:26:16Z

...eoMechanicsApplication/custom_processes/apply_constant_interpolate_line_pressure_process.cpp

        << "No boundary node is found for interpolate line pressure process" << std::endl;
 }

 bool ApplyConstantInterpolateLinePressureProcess::IsMoreThanOneElementWithThisEdgeFast(


Thanks for refactoring this function slightly, such that the complexity is lower. I think it can be simplified even more, but that's out of scope of this PR. For that I'd like to first add unit tests and then start simplifying (and seeing how small +clear we could get this code).

That's for later, thanks for the efforts here!

rfaasse · 2026-01-16T08:26:58Z

.../GeoMechanicsApplication/custom_processes/apply_constant_interpolate_line_pressure_process.h

+/// output stream function
+inline std::ostream& operator<<(std::ostream& rOStream, const ApplyConstantInterpolateLinePressureProcess& rThis)
+{
+    rThis.PrintInfo(rOStream);
+    rOStream << std::endl;
+    rThis.PrintData(rOStream);
+
+    return rOStream;
+}


That's also a solution, fine with me!

markelov208 · 2026-01-16T08:45:14Z

Hi Richard, many thanks for the review and comments. They are in Kratos Product Backlog (view) for future work.

markelov208 added 6 commits December 18, 2025 12:29

split apply_boundary_hydrostatic_pressure_table_process

49e2a67

split apply_boundary_phreatic_line_pressure_table_process

0d9683c

split apply_boundary_phreatic_surface_pressure_table_process

5e452b0

split apply_constant_boundary_hydrostatic_pressure_process

194b190

split apply_constant_boundary_phreatic_line_pressure_process

87599ca

split apply_constant_boundary_phreatic_surface_pressure_process

e92a251

markelov208 self-assigned this Dec 18, 2025

markelov208 requested a review from a team as a code owner December 18, 2025 12:44

markelov208 added 17 commits December 18, 2025 17:32

split apply_constant_hydrostatic_pressure_process.hpp

9ac4ed7

split apply_constant_interpolate_line_pressure_process.hpp

f5cb0c7

split apply_constant_phreatic_line_pressure_process.hpp

9dd191a

used forward declaration for apply_constant_phreatic_line_pressure_pr…

eec7b63

…ocess

moved inline functions to header for apply_constant_interpolate_line_…

f848b23

…pressure_process

split apply_constant_phreatic_surface_pressure_process.hpp

73ff850

splt apply_hydrostatic_pressure_table_process

54080a9

split apply_phreatic_line_pressure_table_process.hpp

545d67f

split apply_phreatic_surface_pressure_table_process.hpp

c60ecef

split apply_write_result_scalar_process.hpp

e9fccd3

cleaned deactivate_conditions_on_inactive_elements_process.hpp

3d99fcb

renamed geo_extrapolate_integration_point_values_to_nodes_process.h a…

a0458db

…s hpp

split set_absorbing_boundary_parameters_process.hpp

c057a1c

cleaned includes

6c4fcc7

added forward declaration for ModelPart

2a18206

added forward declaration for Parameters

260d5e4

added using namespace std::string_literals;

f86a966

markelov208 requested review from avdg81 and rfaasse December 19, 2025 15:34

WPK4FEM linked an issue Dec 22, 2025 that may be closed by this pull request

[GeoMechanicsApplication] Move all non-template code to .cpp files and make sure any .h file does not contain any implementations #14046

Closed

Merge branch 'master' into geo/14046-move-non-template-code-from-h-to…

b381bbc

…-cpp-processes

rfaasse reviewed Dec 24, 2025

View reviewed changes

markelov208 added 18 commits December 24, 2025 12:32

moved default destructors to headers

322fdd1

reduced code smells

5d61d92

response to the review comments and code smells

bc19a43

moved include <string> to headers

7a6dd17

reverted changes for apply_constant_interpolate_line_pressure_process

bcbd92f

removed some of code smells

ecdf793

used auto

9d74fa8

reduced complexity by using continue

4265353

small reduction of the complexity

90856ad

introduced lamda ContainsElementInRange to reduce complexity

338024f

minor changes

7baf55a

clang format

18d4dfb

Merge branch 'master' into geo/14046-move-non-template-code-from-h-to…

4c1ab9b

…-cpp-processes

moved lambda out of for loop

028099a

used static_cast<int> to remove a code smell

4eed7d8

Merge branch 'master' into geo/14046-move-non-template-code-from-h-to…

b602cb2

…-cpp-processes # Conflicts: # applications/GeoMechanicsApplication/custom_workflows/dgeoflow.h # applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp

removed operators

09808cb

removed inline operators

405f8e7

markelov208 requested a review from rfaasse January 15, 2026 22:19

rfaasse approved these changes Jan 16, 2026

View reviewed changes

markelov208 merged commit 1db7fd0 into master Jan 16, 2026
9 of 10 checks passed

markelov208 deleted the geo/14046-move-non-template-code-from-h-to-cpp-processes branch January 16, 2026 09:15

[GeoMechanicsApplication] Move all non-template code to .cpp files and make sure any .h file does not contain any implementations (processes) #14075

[GeoMechanicsApplication] Move all non-template code to .cpp files and make sure any .h file does not contain any implementations (processes) #14075

Uh oh!

Conversation

markelov208 commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rfaasse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rfaasse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markelov208 commented Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

markelov208 commented Dec 18, 2025 •

edited

Loading